简读clubof网站源码之后的思考
注:本文所阅读的clubof源码版本为FrienDevSourceCode_20081028,即2008年10月28日。
按说昨天刚参加“微软技术创新日--北京站”活动之后, 今天就来评论其活动中产品的一些问题显
得不太厚道。但本文内容绝不应当看作是关于clubof的负面评论,并且可以说我是绝对支持这个网站
并响应其所“开放源码”这一举措的。甚至我还把该网站的宣传贴纸贴到了我的笔记本上。
因为这是国内第一个开源并已上线动作的基于.net的SNS软件产品,更是先后有10名微软工程师
耗时三个月心血所开发出的产品。所以光就这一点来说,在国内的.net开发者中就应该会很有影响力,
甚至可能说是“从某种意义上传达出了微软内部开发者的一些声音”。
报着这样的心态,我下载并开始阅读其源码,只不过是在阅读源码的过程中看到了一些有意思的
问题,其中还伴随着自己的一些思考。这里再次重申一下,就是我希望看到该产品发展的越来越好,
从而给国内的开发者竖立一个优秀的榜样。
好了,下面就开始看一下我从源码中找出的一些问题。
1.代码重复:
位于FrienDevWeb项目下的"/Club/ActivityList.ascx"
位于FrienDevWeb项目下的"/Club/ClubPostsList.ascx"
重复代码段如下:
建议:将这类代码重构到一个工具类当中,并以静态方法声明以方便调用。
2.在ascx为后缀的cs文件中,存在类(实体)信息定义,比如:
FrienDevWeb/Club/Activity/FeedBackSummary.ascx.cs
建议:可以新创建一个文件夹甚至项目来统一管理这种类实体信息。
3. 业务逻辑被放置到了页面Page_Load之中,比如:
FrienDevWeb/Club/Activity/FeedBackSummary.ascx.cs:
上面代码段中的“IEnumerable<User> users = ActivityRules.Instance...” 开始,到foreach循环结束,
我感觉这块代码应该被放在业务逻辑层中,而Page_Load方法要做的事就是获取表单数据和相应控件的信息绑定
即可,如果像这样把业务逻辑代码强加到当前方法后,会造成当前page_load的(业务)流程代码过多,造成代码阅
读的困难。
4. 方法定义后却未被使用,怀疑是“代码拷贝”的恶果,参见:ClubInviteService.cs中有对上面的
GetQueryStringValue等私有方法的声明。
5. WebService中的实体类声明倒底该不该放到相应的服务类定义中,我想这个问题大家可能会有不同意见,
但我的想法是提出来放到一个公共的entity或信息类项目中,这样会更清楚,不是吗?
出现这个情况的是FrienDevWeb/App_Code/SelectFriendsService.cs,代码如下:
当然这几个小问题不会影响系统的稳定性或性能,只不过是造成代码阅读和项目分布理解上的困扰。
下面就到了我所认为的分层问题了,其实看过MVCStore_Preview1A源码的人可能会发现一些情况,
就是其对Service的引入和使用,这些Service是封装的是业务逻辑,这类逻辑的数据访问代码并不是直
接访问LINQ设计器所生成的类或方法,而是又封装了一层,而这一层的封装有统一的命名规范,即添加
后缀“Repository”,这类做的好处应该是将频繁使用的数据访问方法先抽出来加以封装,比如说订单
等,而Service层会对这类频繁访问的代码直接使用,以确保Service只加关心业务逻辑层的实现,我想
MVCStore的做法是可以认真考虑并加以参考的。比如说:
当然MVCStore这种数据获取方式有个问题,就是效率,即_orderRepository.GetOrders()先是返回所有定单,
而不是仅获取符合条件的数据,并且在后续方法中访问CurrentOrderForUser方法来传入条件 。
注:当然这个判断可能会有误差,就是LINQ中如果不使用tolist这类方法时,查询是不会被马上执行的,这个技
术好像叫lazyload,如果上面两个方法(GetOrders方法和CurrentOrderForUser方法)的调用只是在将linq语法转
(翻)译成sql语法的话,上面的观点就不成立了,因为在Service中代码的最后会调用SingleOrDefault(),如下:
_orderRepository.GetOrders().CurrentOrderForUser(userName).SingleOrDefault();
也就是在该方法(SingleOrDefault)调用时,才会真正将转义过来的sql语句加以执行(向数据库递交查询请求)。
当然这也是个猜测,如果有对LINQ研究的比较深入的朋友可以测试一下,然后告之一下我,以免我的判断出
现误差,在此先行道谢了。
下面再看看在业务规则层中的一个小问题:)
有关于单体(singleton)模式的实现方法,我看到了这样的代码(位于FriendRules):
public static FriendRules Instance = new FriendRules();
private FrienDevDataContext m_DataContext = FrienDevDataContext.Instance;
private FriendRules()
{
}
..
然而在UserRules.cs文件中又出现了下面这种单体模式的“经典实现”代码:
看来在如何实现单体模式上,团队的开发者中还是有不同意见和编写方式的:)
不过如果使用下面这种方式的话,可以说会造成一个并发访问的问题,而这个问题以前在网上就有
人提出过并给出了一个方案,这里就不再多说了。
建议将上面的UserRules代码修改成:
好了,限于尚处于刚开始阅读clubof代码,对该项目的代码还远远谈不上了解,今天的内容就先到这里了。
按说昨天刚参加“微软技术创新日--北京站”活动之后, 今天就来评论其活动中产品的一些问题显
得不太厚道。但本文内容绝不应当看作是关于clubof的负面评论,并且可以说我是绝对支持这个网站
并响应其所“开放源码”这一举措的。甚至我还把该网站的宣传贴纸贴到了我的笔记本上。
因为这是国内第一个开源并已上线动作的基于.net的SNS软件产品,更是先后有10名微软工程师
耗时三个月心血所开发出的产品。所以光就这一点来说,在国内的.net开发者中就应该会很有影响力,
甚至可能说是“从某种意义上传达出了微软内部开发者的一些声音”。
报着这样的心态,我下载并开始阅读其源码,只不过是在阅读源码的过程中看到了一些有意思的
问题,其中还伴随着自己的一些思考。这里再次重申一下,就是我希望看到该产品发展的越来越好,
从而给国内的开发者竖立一个优秀的榜样。
好了,下面就开始看一下我从源码中找出的一些问题。
1.代码重复:
位于FrienDevWeb项目下的"/Club/ActivityList.ascx"
位于FrienDevWeb项目下的"/Club/ClubPostsList.ascx"
重复代码段如下:
#region Protected Method
protected int GetQueryStringValue(string QueryName, int nullValue)
{
if (string.IsNullOrEmpty(this.Page.Request.QueryString[QueryName]))
{
return nullValue;
}
return Convert.ToInt32(this.Page.Request.QueryString[QueryName]);
}
protected string GetQueryStringValue(string QueryName, string nullValue)
{
if (string.IsNullOrEmpty(this.Page.Request.QueryString[QueryName]))
{
return nullValue;
}
return this.Page.Request.QueryString[QueryName];
}
protected bool GetQueryStringValue(string QueryName, bool nullValue)
{
if (string.IsNullOrEmpty(this.Page.Request.QueryString[QueryName]))
{
return nullValue;
}
return Convert.ToBoolean(this.Page.Request.QueryString[QueryName]);
}
#endregion
protected int GetQueryStringValue(string QueryName, int nullValue)
{
if (string.IsNullOrEmpty(this.Page.Request.QueryString[QueryName]))
{
return nullValue;
}
return Convert.ToInt32(this.Page.Request.QueryString[QueryName]);
}
protected string GetQueryStringValue(string QueryName, string nullValue)
{
if (string.IsNullOrEmpty(this.Page.Request.QueryString[QueryName]))
{
return nullValue;
}
return this.Page.Request.QueryString[QueryName];
}
protected bool GetQueryStringValue(string QueryName, bool nullValue)
{
if (string.IsNullOrEmpty(this.Page.Request.QueryString[QueryName]))
{
return nullValue;
}
return Convert.ToBoolean(this.Page.Request.QueryString[QueryName]);
}
#endregion
建议:将这类代码重构到一个工具类当中,并以静态方法声明以方便调用。
2.在ascx为后缀的cs文件中,存在类(实体)信息定义,比如:
FrienDevWeb/Club/Activity/FeedBackSummary.ascx.cs
class CustomQuestionAnswers
{
public string UserHomeUrl { get; set; }
public string Name { get; set; }
public string Answer { get; set; }
public string Question { get; set; }
}
{
public string UserHomeUrl { get; set; }
public string Name { get; set; }
public string Answer { get; set; }
public string Question { get; set; }
}
建议:可以新创建一个文件夹甚至项目来统一管理这种类实体信息。
3. 业务逻辑被放置到了页面Page_Load之中,比如:
FrienDevWeb/Club/Activity/FeedBackSummary.ascx.cs:
protected void Page_Load(object sender, EventArgs e)
{
string activityIdString = Request["activityId"];
if (!int.TryParse(activityIdString, out m_activityId))
Response.Redirect("~/Home/Default.aspx");
if (!ActivityRules.Instance.IsCurrentUserIsActivityPromoter(m_activityId))
Response.Redirect("~/Home/Default.aspx");
//下面是业务逻辑代码
IEnumerable<User> users = ActivityRules.Instance.GetActivityJoined(m_activityId, -1);
List<string> userIds = new List<string>();
foreach (User user in users)
{
userIds.Add(user.UserId);
}
List<int[]> marks = ActivityRules.Instance.GetUsersFeedBackMark(m_activityId, userIds);
int markCount = marks.Count != 0 ? marks.Count : 1;
int overallSum = 0;
int contentSum = 0;
int organizeSum = 0;
foreach (int[] mark in marks)
{
overallSum += mark[0];
contentSum += mark[1];
organizeSum += mark[2];
}
//业务逻辑代码结束
lblOverallMark.Text = overallSum.ToString();
lblContentMark.Text = contentSum.ToString();
lblOrganizeMark.Text = organizeSum.ToString();
lblOverallAverage.Text = (overallSum / markCount).ToString();
lblContentAverage.Text = (contentSum / markCount).ToString();
lblOrganizeAverage.Text = (organizeSum / markCount).ToString();
}
{
string activityIdString = Request["activityId"];
if (!int.TryParse(activityIdString, out m_activityId))
Response.Redirect("~/Home/Default.aspx");
if (!ActivityRules.Instance.IsCurrentUserIsActivityPromoter(m_activityId))
Response.Redirect("~/Home/Default.aspx");
//下面是业务逻辑代码
IEnumerable<User> users = ActivityRules.Instance.GetActivityJoined(m_activityId, -1);
List<string> userIds = new List<string>();
foreach (User user in users)
{
userIds.Add(user.UserId);
}
List<int[]> marks = ActivityRules.Instance.GetUsersFeedBackMark(m_activityId, userIds);
int markCount = marks.Count != 0 ? marks.Count : 1;
int overallSum = 0;
int contentSum = 0;
int organizeSum = 0;
foreach (int[] mark in marks)
{
overallSum += mark[0];
contentSum += mark[1];
organizeSum += mark[2];
}
//业务逻辑代码结束
lblOverallMark.Text = overallSum.ToString();
lblContentMark.Text = contentSum.ToString();
lblOrganizeMark.Text = organizeSum.ToString();
lblOverallAverage.Text = (overallSum / markCount).ToString();
lblContentAverage.Text = (contentSum / markCount).ToString();
lblOrganizeAverage.Text = (organizeSum / markCount).ToString();
}
上面代码段中的“IEnumerable<User> users = ActivityRules.Instance...” 开始,到foreach循环结束,
我感觉这块代码应该被放在业务逻辑层中,而Page_Load方法要做的事就是获取表单数据和相应控件的信息绑定
即可,如果像这样把业务逻辑代码强加到当前方法后,会造成当前page_load的(业务)流程代码过多,造成代码阅
读的困难。
4. 方法定义后却未被使用,怀疑是“代码拷贝”的恶果,参见:ClubInviteService.cs中有对上面的
GetQueryStringValue等私有方法的声明。
5. WebService中的实体类声明倒底该不该放到相应的服务类定义中,我想这个问题大家可能会有不同意见,
但我的想法是提出来放到一个公共的entity或信息类项目中,这样会更清楚,不是吗?
出现这个情况的是FrienDevWeb/App_Code/SelectFriendsService.cs,代码如下:
//注:代码上方为SelectFriendsService类的定义
public class UserItem
{
public UserItem()
{ }
public UserItem(string UserId, string FullName, string PhotoFileName)
{
this.UserId = UserId;
this.FullName = FullName;
this.PhotoFileName = PhotoFileName;
}
private string m_UserId;
public string UserId
{
..
}
private string m_FullName;
public string FullName
{
..
}
private string m_PhotoFileName;
public string PhotoFileName
{
..
}
}
public class UserItem
{
public UserItem()
{ }
public UserItem(string UserId, string FullName, string PhotoFileName)
{
this.UserId = UserId;
this.FullName = FullName;
this.PhotoFileName = PhotoFileName;
}
private string m_UserId;
public string UserId
{
..
}
private string m_FullName;
public string FullName
{
..
}
private string m_PhotoFileName;
public string PhotoFileName
{
..
}
}
当然这几个小问题不会影响系统的稳定性或性能,只不过是造成代码阅读和项目分布理解上的困扰。
下面就到了我所认为的分层问题了,其实看过MVCStore_Preview1A源码的人可能会发现一些情况,
就是其对Service的引入和使用,这些Service是封装的是业务逻辑,这类逻辑的数据访问代码并不是直
接访问LINQ设计器所生成的类或方法,而是又封装了一层,而这一层的封装有统一的命名规范,即添加
后缀“Repository”,这类做的好处应该是将频繁使用的数据访问方法先抽出来加以封装,比如说订单
等,而Service层会对这类频繁访问的代码直接使用,以确保Service只加关心业务逻辑层的实现,我想
MVCStore的做法是可以认真考虑并加以参考的。比如说:
/// 位于MVCStore_Preview1A代码的SqlOrderRepository.cs中
/// <summary>
/// Returns a current, unchecked-out order for the current user
/// </summary>
public Order GetCurrentOrder(string userName)
{
Order result= _orderRepository.GetOrders().CurrentOrderForUser(userName).SingleOrDefault();
if(result==null)
{
//create a new one
result=new Order(userName);
}
return result;
}
//Commerce.Data.SqlCatalogRepository
public IQueryable<Order> GetOrders() {
var orders = from o in _db.Orders
let items = GetOrderItems(o.OrderID)
let transactions = GetTransactions(o.OrderID)
select new Order
{
Status = (OrderStatus)o.OrderStatusID,
DateCreated = o.CreatedOn,
ID = o.OrderID,
OrderNumber = o.OrderNumber,
Items = new LazyList<OrderItem>(items),
Transactions = new LazyList<Transaction>(transactions),
ShippingAddress = GetAddresses().Where(x => x.ID == o.ShippingAddressID).SingleOrDefault(),
BillingAddress = GetAddresses().Where(x => x.ID == o.BillingAddressID).SingleOrDefault(),
ShippingMethod =GetOrderShippingMethod(o.ShippingMethod, o),
UserName = o.UserName,
UserLanguageCode=o.UserLanguageCode,
DateShipped=o.DateShipped,
EstimatedDelivery = o.EstimatedDelivery,
TrackingNumber=o.TrackingNumber,
TaxAmount=o.TaxAmount,
DiscountReason=o.DiscountReason,
DiscountAmount=o.DiscountAmount
};
return orders;
}
///该方法被放在了Commerce.Data.OrderFilters中,但我感觉这块代码应与上面代码放在一起
public static IQueryable<Order> CurrentOrderForUser(this IQueryable<Order> qry, string userName)
{
return from o in qry
where o.UserName == userName && o.Status==OrderStatus.NotCheckoutOut
select o;
}
/// <summary>
/// Returns a current, unchecked-out order for the current user
/// </summary>
public Order GetCurrentOrder(string userName)
{
Order result= _orderRepository.GetOrders().CurrentOrderForUser(userName).SingleOrDefault();
if(result==null)
{
//create a new one
result=new Order(userName);
}
return result;
}
//Commerce.Data.SqlCatalogRepository
public IQueryable<Order> GetOrders() {
var orders = from o in _db.Orders
let items = GetOrderItems(o.OrderID)
let transactions = GetTransactions(o.OrderID)
select new Order
{
Status = (OrderStatus)o.OrderStatusID,
DateCreated = o.CreatedOn,
ID = o.OrderID,
OrderNumber = o.OrderNumber,
Items = new LazyList<OrderItem>(items),
Transactions = new LazyList<Transaction>(transactions),
ShippingAddress = GetAddresses().Where(x => x.ID == o.ShippingAddressID).SingleOrDefault(),
BillingAddress = GetAddresses().Where(x => x.ID == o.BillingAddressID).SingleOrDefault(),
ShippingMethod =GetOrderShippingMethod(o.ShippingMethod, o),
UserName = o.UserName,
UserLanguageCode=o.UserLanguageCode,
DateShipped=o.DateShipped,
EstimatedDelivery = o.EstimatedDelivery,
TrackingNumber=o.TrackingNumber,
TaxAmount=o.TaxAmount,
DiscountReason=o.DiscountReason,
DiscountAmount=o.DiscountAmount
};
return orders;
}
///该方法被放在了Commerce.Data.OrderFilters中,但我感觉这块代码应与上面代码放在一起
public static IQueryable<Order> CurrentOrderForUser(this IQueryable<Order> qry, string userName)
{
return from o in qry
where o.UserName == userName && o.Status==OrderStatus.NotCheckoutOut
select o;
}
当然MVCStore这种数据获取方式有个问题,就是效率,即_orderRepository.GetOrders()先是返回所有定单,
而不是仅获取符合条件的数据,并且在后续方法中访问CurrentOrderForUser方法来传入条件 。
注:当然这个判断可能会有误差,就是LINQ中如果不使用tolist这类方法时,查询是不会被马上执行的,这个技
术好像叫lazyload,如果上面两个方法(GetOrders方法和CurrentOrderForUser方法)的调用只是在将linq语法转
(翻)译成sql语法的话,上面的观点就不成立了,因为在Service中代码的最后会调用SingleOrDefault(),如下:
_orderRepository.GetOrders().CurrentOrderForUser(userName).SingleOrDefault();
也就是在该方法(SingleOrDefault)调用时,才会真正将转义过来的sql语句加以执行(向数据库递交查询请求)。
当然这也是个猜测,如果有对LINQ研究的比较深入的朋友可以测试一下,然后告之一下我,以免我的判断出
现误差,在此先行道谢了。
下面再看看在业务规则层中的一个小问题:)
有关于单体(singleton)模式的实现方法,我看到了这样的代码(位于FriendRules):
public static FriendRules Instance = new FriendRules();
private FrienDevDataContext m_DataContext = FrienDevDataContext.Instance;
private FriendRules()
{
}
..
然而在UserRules.cs文件中又出现了下面这种单体模式的“经典实现”代码:
private static UserRules _instance;
public static UserRules Instance
{
get
{
if (_instance == null)
{
_instance = new UserRules();
}
return _instance;
}
}
public static UserRules Instance
{
get
{
if (_instance == null)
{
_instance = new UserRules();
}
return _instance;
}
}
看来在如何实现单体模式上,团队的开发者中还是有不同意见和编写方式的:)
不过如果使用下面这种方式的话,可以说会造成一个并发访问的问题,而这个问题以前在网上就有
人提出过并给出了一个方案,这里就不再多说了。
建议将上面的UserRules代码修改成:
public static UserRules Instance
{
get
{
if (_instance == null)
{
lock (lockHelper)
{
if (_instance == null)
{
_instance = new UserRules();
}
}
}
return _instance;
}
}
{
get
{
if (_instance == null)
{
lock (lockHelper)
{
if (_instance == null)
{
_instance = new UserRules();
}
}
}
return _instance;
}
}
好了,限于尚处于刚开始阅读clubof代码,对该项目的代码还远远谈不上了解,今天的内容就先到这里了。
最后希望国内.net开发的所有软件产品越做越好,市场越来越大。
原文链接:http://www.cnblogs.com/daizhj/archive/2009/03/04/1402708.html
作者: daizhj, 代震军
Tags: clubof,source code, 源代码
网址: http://daizhj.cnblogs.com/